Skip to content

feat(transformer): add ast changed hint in InjectGlobalVariables#7205

Closed
IWANABETHATGUY wants to merge 2 commits intomainfrom
feat/changed-hint-in-oxc-transform-plugin
Closed

feat(transformer): add ast changed hint in InjectGlobalVariables#7205
IWANABETHATGUY wants to merge 2 commits intomainfrom
feat/changed-hint-in-oxc-transform-plugin

Conversation

@IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Nov 8, 2024

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 8, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Nov 8, 2024
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review November 8, 2024 08:05
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 8, 2024

CodSpeed Performance Report

Merging #7205 will not alter performance

Comparing feat/changed-hint-in-oxc-transform-plugin (b2ab578) with main (4a515be)

Summary

✅ 30 untouched benchmarks

@Boshen Boshen changed the title chore(oxc_transformer): add ast changed hint in oxc_transformer feat(transformer): add ast changed hint in InjectGlobalVariables Nov 8, 2024
@github-actions github-actions bot added the C-enhancement Category - New feature or request label Nov 8, 2024
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need some tests.

@IWANABETHATGUY
Copy link
Contributor Author

For every test, I add an extra assert to make sure if the ast changed, the XXXXReturn.changed is also needs to be true

@IWANABETHATGUY IWANABETHATGUY force-pushed the feat/changed-hint-in-oxc-transform-plugin branch from 1e9ba48 to b2ab578 Compare November 8, 2024 09:05
@IWANABETHATGUY
Copy link
Contributor Author

Seems noise, same as #7203 (comment)

@Dunqing
Copy link
Member

Dunqing commented Nov 8, 2024

It seems the plugin is not very complex, I suggest syncing the semantic rather than adding a state to indicate it's no change.

@IWANABETHATGUY
Copy link
Contributor Author

IWANABETHATGUY commented Nov 8, 2024

It seems the plugin is not very complex, I suggest syncing the semantic rather than adding a state to indicate it's no change.

@IWANABETHATGUY IWANABETHATGUY deleted the feat/changed-hint-in-oxc-transform-plugin branch November 8, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants